Skip to content

fix: _Installation update fails when clearing deviceToken#10454

Open
AdrianCurtin wants to merge 1 commit intoparse-community:alphafrom
AdrianCurtin:fix_installation_device_token_clear
Open

fix: _Installation update fails when clearing deviceToken#10454
AdrianCurtin wants to merge 1 commit intoparse-community:alphafrom
AdrianCurtin:fix_installation_device_token_clear

Conversation

@AdrianCurtin
Copy link
Copy Markdown

@AdrianCurtin AdrianCurtin commented May 4, 2026

Issue

Updating an existing _Installation to clear deviceToken from the client (iOS/Android SDKs, Dashboard, anything sending { deviceToken: { __op: 'Delete' } } or { deviceToken: null }) fails with:

You cannot use [object Object] as a query parameter. {"code":107}
    at transformQueryKeyValue (Adapters/Storage/Mongo/MongoTransform.js:389:11)
    at transformWhere (Adapters/Storage/Mongo/MongoTransform.js:399:17)
    at MongoStorageAdapter.find (Adapters/Storage/Mongo/MongoStorageAdapter.js:580:59)

RestWrite.handleInstallation runs at step 109 of RestWrite.execute, before the field-write loop processes __op operators. The Delete operator object is truthy, so it was being pushed into the $or lookup query as { deviceToken: { __op: 'Delete' } }, which transformQueryKeyValue cannot transform into a Mongo predicate.

Approach

Detect the two clearing shapes (null and { __op: 'Delete' }) at the top of handleInstallation and derive a deviceTokenForLookup value (undefined when clearing). Identification paths use deviceTokenForLookup:

  • the "must specify ID" guard (only matters for create — clearing alone with no other identifier still throws 135)
  • the 64-char iOS lowercase normalization
  • the "no critical change" early-return for queries
  • the $or lookup orQueries push
  • the result.deviceToken match loop
  • the "deviceToken may not be changed" 136 throw
  • both deviceToken-conflict cleanup delQuery blocks

this.data.deviceToken is left untouched, so the actual write step still applies the Delete op and the field is cleared on the row.

Tasks

  • Add tests
  • Add changes to documentation (guides, repository pages, code comments)
  • Add security check
  • Add new Parse Error codes to Parse JS SDK

Summary by CodeRabbit

  • Bug Fixes

    • Fixed device token clearing on iOS installations. Device tokens are now properly removed when explicitly deleted, even during updates to other installation fields.
  • Tests

    • Added tests verifying device token deletion behavior during installation updates.

handleInstallation runs before Parse __op operators are processed, so
when a client cleared deviceToken via { __op: 'Delete' } or null, the
operator object was being pushed into the Mongo $or lookup query and
rejected by transformQueryKeyValue with "You cannot use [object Object]
as a query parameter" (Parse error 107).

Detect the clearing intent and route the identification/lookup paths
through deviceTokenForLookup (undefined when clearing) while leaving
this.data.deviceToken untouched so the field is still cleared on write.

Adds regression specs covering both clearing shapes and the case where
deviceToken is cleared alongside another field update.
Copilot AI review requested due to automatic review settings May 4, 2026 17:35
@parse-github-assistant
Copy link
Copy Markdown

🚀 Thanks for opening this pull request! We appreciate your effort in improving the project. Please let us know once your pull request is ready for review.

Tip

  • Keep pull requests small. Large PRs will be rejected. Break complex features into smaller, incremental PRs.
  • Use Test Driven Development. Write failing tests before implementing functionality. Ensure tests pass.
  • Group code into logical blocks. Add a short comment before each block to explain its purpose.
  • We offer conceptual guidance. Coding is up to you. PRs must be merge-ready for human review.
  • Our review focuses on concept, not quality. PRs with code issues will be rejected. Use an AI agent.
  • Human review time is precious. Avoid review ping-pong. Inspect and test your AI-generated code.

Note

Please respond to review comments from AI agents just like you would to comments from a human reviewer. Let the reviewer resolve their own comments, unless they have reviewed and accepted your commit, or agreed with your explanation for why the feedback was incorrect.

Caution

Pull requests must be written using an AI agent with human supervision. Pull requests written entirely by a human will likely be rejected, because of lower code quality, higher review effort and the higher risk of introducing bugs. Please note that AI review comments on this pull request alone do not satisfy this requirement. Our CI and AI review are safeguards, not development tools. If many issues are flagged, rethink your development approach. Invest more effort in planning and design rather than using review cycles to fix low-quality code.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 4, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e3d7d962-0ae6-4ed9-93b9-4f0054d92ddb

📥 Commits

Reviewing files that changed from the base of the PR and between 4f90c5e and 7c386e6.

📒 Files selected for processing (2)
  • spec/ParseInstallation.spec.js
  • src/RestWrite.js

📝 Walkthrough

Walkthrough

This PR fixes how Parse Server handles deviceToken clearing in _Installation updates. It distinguishes between "no deviceToken provided" and "deviceToken explicitly being cleared" by introducing a deviceTokenForLookup variable that remains undefined when deviceToken is set to null or { __op: 'Delete' }. Three new tests validate the behavior across different clearing scenarios.

Changes

Installation DeviceToken Clearing Logic

Layer / File(s) Summary
Core Logic & Clearing Detection
src/RestWrite.js (lines 1298–1311)
Introduces clearingDeviceToken flag to detect when deviceToken is null or { __op: 'Delete' }, and derives deviceTokenForLookup as undefined when clearing, otherwise using the actual token value.
Token Normalization
src/RestWrite.js (lines 1322–1326)
Lowercases and syncs deviceTokenForLookup to this.data.deviceToken only when a real lookup token exists, preventing operator objects from being normalized.
Query & Validation Integration
src/RestWrite.js (lines 1344–1347, 1367–1370, 1411–1416, 1465–1468, 1500–1506)
Updates early-return conditions, _Installation $or lookups, conflict validation, and cleanup delete queries to use deviceTokenForLookup instead of this.data.deviceToken, ensuring operator objects are never used in Mongo operations.
Test Validation
spec/ParseInstallation.spec.js (lines 547–649)
Adds three tests verifying deviceToken clearing via { __op: 'Delete' }, null, and combined with other field updates, confirming the token is removed while other fields and installationId are preserved.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~28 minutes

Possibly related PRs

Suggested reviewers

  • mtrezza
🚥 Pre-merge checks | ✅ 6 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Engage In Review Feedback ❓ Inconclusive The repository sandbox cannot access GitHub PR #10454's review infrastructure to verify reviewer feedback, user engagement, or resolution documentation. Access GitHub PR #10454 at #10454 and review all feedback comments and user responses in the review thread.
✅ Passed checks (6 passed)
Check name Status Explanation
Title check ✅ Passed The pull request title correctly follows the required format with 'fix:' prefix and clearly describes the main issue being resolved.
Description check ✅ Passed The pull request description comprehensively covers all required sections: Issue (with error details), Approach (with implementation strategy), and Tasks (with completed and skipped items marked).
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Security Check ✅ Passed The PR properly addresses a Mongo Query Injection vulnerability by introducing a deviceTokenForLookup variable set to undefined when clearing tokens, preventing operator objects from reaching Mongo transforms.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 OpenGrep (1.20.0)

OpenGrep fatal error (exit code 2): [00.18][ERROR]: Error: exception Unix_error: No such file or directory stat spec/ParseInstallation.spec.js
Raised by primitive operation at UTmp.replace_named_pipe_by_regular_file_if_needed in file "libs/commons/UTmp.ml", line 145, characters 8-27
Called from Scan_CLI.replace_target_roots_by_regular_files_where_needed.(fun) in file "src/osemgrep/cli_scan/Scan_CLI.ml", lines 1086-1087, characters 19-65
Called from List_.fast_map in file "libs/commons/List_.ml", line 81, characters 17-20
Called fr


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates _Installation handling so clearing deviceToken during an update no longer feeds a delete/null payload into the installation lookup path, which was causing Mongo query transformation failures.

Changes:

  • Adds a deviceTokenForLookup path in RestWrite.handleInstallation to treat null and { __op: 'Delete' } as non-identifying values during installation matching.
  • Updates installation matching, validation, and dedup lookup branches to use the derived lookup token instead of the raw request payload.
  • Adds regression tests covering deviceToken clearing via delete op, via null, and while updating another field.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/RestWrite.js Adjusts _Installation lookup logic to avoid querying with delete-op/null deviceToken values.
spec/ParseInstallation.spec.js Adds regression tests for clearing deviceToken during installation updates.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/RestWrite.js
Comment on lines +1299 to +1305
// Treat that as "no deviceToken to identify/match by" so we do not feed the
// operator object into the lookup query (which would fail Mongo transform).
const clearingDeviceToken =
this.data.deviceToken === null ||
(typeof this.data.deviceToken === 'object' &&
this.data.deviceToken !== null &&
this.data.deviceToken.__op === 'Delete');
Comment on lines +605 to +606
expect(results[0].deviceToken == null).toBeTrue();
expect(results[0].installationId).toEqual(installId);
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants